Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More accurate spans, season 4 #12181

Merged
merged 1 commit into from
Feb 11, 2014
Merged

Conversation

sanxiyn
Copy link
Member

@sanxiyn sanxiyn commented Feb 11, 2014

No description provided.

@sanxiyn
Copy link
Member Author

sanxiyn commented Feb 11, 2014

Before:

span.rs:6         self.a
                  ^~~~~
span.rs:10     let s = S { a: 0 };
                       ^

After:

span.rs:6         self.a
                  ^~~~
span.rs:10     let s = S { a: 0 };
                       ^~~~~~~~~~

@bors bors closed this Feb 11, 2014
@bors bors merged commit f3b5ec2 into rust-lang:master Feb 11, 2014
@sanxiyn sanxiyn deleted the accurate-span-4 branch February 12, 2014 00:40
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 8, 2024
[`unconditional_recursion`]: compare by `Ty`s instead of `DefId`s

Fixes rust-lang#12154
Fixes rust-lang#12181 (this was later edited in, so the rest of the description refers to the first linked issue)

Before this change, the lint would work with `DefId`s and use those to compare types. This PR changes it to compare types directly. It fixes the linked issue, but also other false positives I found in a lintcheck run. For example, one of the issues is that some types don't have `DefId`s (primitives, references, etc., leading to possible FNs), and the helper function used to extract a `DefId` didn't handle type parameters.

Another issue was that the lint would use `.peel_refs()` in a few places where that could lead to false positives (one such FP was in the `http` crate). See the doc comment on one of the added functions and also the test case for what I mean.

The code in the linked issue was linted because the receiver type is `T` (a `ty::Param`), which was not handled in `get_ty_def_id` and returned `None`, so this wouldn't actually *get* to comparing `self_arg != ty_id` here, and skip the early-return:
https://github.com/rust-lang/rust-clippy/blob/70573af31eb9b8431c2e7923325c82ba0304cbb2/clippy_lints/src/unconditional_recursion.rs#L171-L178

This alone could be fixed by doing something like `&& get_ty_def_id(ty).map_or(true, |ty_id)| self_arg != ty_id)`, but we don't really need to work with `DefId`s in the first place, I don't think.

changelog: [`unconditional_recursion`]: avoid linting when the other comparison type is a type parameter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants